Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CDRIVER-5835 Rename set_error to kms_set_error #1811

Merged

Conversation

MonkeybreadSoftware
Copy link
Contributor

set_error is an awful generic name for a global function.

Sadly this easily conflicts with other libraries if you statically link this into an application.

please have us rename set_error to kms_set_error to avoid link errors with libraries/applications that also have set_error as function.

@kevinAlbs
Copy link
Collaborator

Thank you for the contribution @MonkeybreadSoftware. The change LGTM. src/kms-message is vendored from libmongocrypt. I would rather make the change in libmongocrypt first, then sync the source in the C driver. The C driver tests check the copy of src/kms-message does not diverge from libmongocrypt. If you are OK making the change in libmongocrypt, please do. Otherwise, we can make that change.

@MonkeybreadSoftware
Copy link
Contributor Author

Thanks.

I made a pull request over there:
mongodb/libmongocrypt#923

I appreciate if you apply this and for future mongo-c-driver versions, I don't need to patch this locally.

@kevinAlbs kevinAlbs self-requested a review December 16, 2024 16:56
@kevinAlbs kevinAlbs changed the title Renamed set_error to kms_set_error Rename set_error to kms_set_error Dec 16, 2024
@kevinAlbs kevinAlbs changed the title Rename set_error to kms_set_error CDRIVER-5835 Rename set_error to kms_set_error Dec 16, 2024
Copy link
Collaborator

@kevinAlbs kevinAlbs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the fix. Changes were verified with this patch build.

@kevinAlbs kevinAlbs merged commit 9498107 into mongodb:master Dec 16, 2024
1 check was pending
kevinAlbs added a commit that referenced this pull request Dec 16, 2024
Update kms-message source to commit e07091d to include rename.

---------

Co-authored-by: Kevin Albertson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants